Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RPC] Add endpoint for fetching CoinMetadata #6281

Merged
merged 1 commit into from
Nov 28, 2022
Merged

Conversation

666lcz
Copy link
Contributor

@666lcz 666lcz commented Nov 21, 2022

Add convenience endpoints for fetching CoinMetadata

lands after #5635

@github-actions
Copy link
Contributor

github-actions bot commented Nov 21, 2022

💳 Wallet Extension has been built, you can download the packaged extension here: https://github.com/MystenLabs/sui/actions/runs/3561669260#artifacts

@@ -66,7 +66,7 @@ export function useCoinDecimals(coinType?: string | null) {
);
}

return rpc.getCoinDenominationInfo(coinType);
return rpc.getCoinMetadata(coinType);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@666lcz what happens if this method isn’t available? This is currently hardcoded for SUI so I’m worried that this will break sui display for networks that don’t yet support this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I just added the logic to return hardcoded value for SUI if the RPC version is prior to 0.17.0

Copy link
Contributor

@patrickkuo patrickkuo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment on possible optimisation we can consider in future PR.
Otherwise Rust code LGTM

@@ -151,6 +155,78 @@ impl RpcFullNodeReadApiServer for FullNodeApi {
Ok(self.state.dry_exec_transaction(tx_data, txn_digest).await?)
}

async fn get_coin_metadata(&self, coin_type: String) -> RpcResult<SuiCoinMetadata> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks quite expensive? We are pulling out the package object every time we need to get the coin's metadata.

if this is frequently accessed, maybe we can consider creating a mapping in AuthorityPerpetualTables?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants